Skip to content

fix workflow(lint) settings#90

Open
GyeongHoKim wants to merge 5 commits into
mbullington:masterfrom
GyeongHoKim:fix/workflow
Open

fix workflow(lint) settings#90
GyeongHoKim wants to merge 5 commits into
mbullington:masterfrom
GyeongHoKim:fix/workflow

Conversation

@GyeongHoKim
Copy link
Copy Markdown
Contributor

Reproduce

All recent failures of Github actions: lint.yml

Reason

Yellowstone uses commander@13.1.0 which expects node version ">=18", but lint workflow runs on Node 14.21.3
Also, there are many files that does not follow eslint rules.

What I did

  1. update lint.yml to use node version 18, and update old actions such as checkout, seutup node, lint
  2. Fixed all ESLint errors except for unused variables (which I don't fully understand, but maybe they are used somehow in the future).

- Yellostone uses commander@13.1.0 and it expects node version ">=18",
but lint workflow runs on node 14.21.3
- Update all actions to LTS version
- Recent commits updated package-lock.json but workflow uses yarn.lock,
delete yarn.lock.
- fix lint errors(unused variables)
- fix lint error to use const variables for never changed variables
@RogerHardiman
Copy link
Copy Markdown
Collaborator

thank you for the pull request.
Michael made the workflow in Yellowstone and I've not looked at it before now so I will try and understand it and include your changes.
Also thank you for the dist upgrade. I forgot that after the AV1 changes but have just fixed it with the new H266 code.

@RogerHardiman
Copy link
Copy Markdown
Collaborator

Hi @GyeongHoKim
There are several unused variables in the RTP De-Packetizers.
When I write a de-packetizer, I extract all the fields from packed bytes then I find it easier when single stepping or when writing debug logs.
There are also some bit fields in RTP bytes that I don't use now, but could check in the future for example the F Bit and the Z Bit in H266 must both be zero and should report an error if they are not (or maybe Throw) but I don't do the check.

So the lint fix for using an Underscore is great. Thank you

@RogerHardiman
Copy link
Copy Markdown
Collaborator

There is a problem with the lint change for using Underscore as a prefix on unused variables.
@mbullington used underscores for some 'class level variables' which is a common coding style in Python.
So I've got to make code changes to these classes and make sure they are not exposed back via an API before I do the change.

I think there are two places I need to make changes

class RTPPacket {
    private _bufpkt: Buffer; // holds the RTP header (12 bytes) AND the RTP packet payload

and

export default class RTSPClient extends EventEmitter {
  username: string;
  password: string;
  headers: { [key: string]: string };

  isConnected = false;
  closed = false;

  // These are all set in #connect or #_netConnect.

  _url?: string;
  _client?: SocketUnion;
  _cSeq = 0;
  _unsupportedExtensions?: string[];
  _authOpions?: AuthOptions;
  // Example: 'SessionId'[';timeout=seconds']
  _session?: string;
  _keepAliveID?: NodeJS.Timeout;
  _nextFreeInterleavedChannel = 0;
  _nextFreeUDPPort = 5000;

and RTSPClient also has some methods that start with Underscore.

I'll have a look.

Roger

- remove underscore prefixes from variable names
- use private keyword instead of underscore prefixes for methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants